-
Notifications
You must be signed in to change notification settings - Fork 965
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial implementation of GitLab OIDC trusted publisher #15275
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! I did a very fast initial pass; will do a more intensive review after.
warehouse/migrations/versions/81f9f9a60270_add_gitlab_oidc_models.py
Outdated
Show resolved
Hide resolved
warehouse/migrations/versions/81f9f9a60270_add_gitlab_oidc_models.py
Outdated
Show resolved
Hide resolved
warehouse/migrations/versions/81f9f9a60270_add_gitlab_oidc_models.py
Outdated
Show resolved
Hide resolved
warehouse/migrations/versions/81f9f9a60270_add_gitlab_oidc_models.py
Outdated
Show resolved
Hide resolved
warehouse/migrations/versions/81f9f9a60270_add_gitlab_oidc_models.py
Outdated
Show resolved
Hide resolved
d53d2a3
to
6d1a5fa
Compare
I suppose it would not be an impossible requirement to say only public users/orgs can be Trusted Publishers. This would however have a practical problem when an existing Trusted Publisher decides to make their org private. Dealing with this might not be worth the trouble. |
Yep, not an impossible requirement, although it would result in an asymmetry vs. GitHub (where any org/repo can publish, whether public or private). |
Most of the ActiveState provider has landed, so this will probably need a decent bit of deconflicting 🙂 |
b3a0d2e
to
4a54f44
Compare
Fixed! |
Am I correct in assuming that we can do this for public namespaces? If so, we should probably attempt to do this, determine if the namespace is public or not, and verify this only for public namespaces. |
Yeah, I think so (@facutuesca can confirm) -- we should be able to do this for public namespaces. |
We could also do this in a TOFU-ish manner for private namespaces, since the OIDC claim set includes the (I'm not sure if this is worth doing, however, since it complicates the relationship between the publisher model and token consumption.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, just some minor comments.
warehouse/migrations/versions/81f9f9a60270_add_gitlab_oidc_models.py
Outdated
Show resolved
Hide resolved
c0bc9b3
to
015714a
Compare
@facutuesca You'll need to re-run |
Co-authored-by: Dustin Ingram <[email protected]>
185c044
to
19d12ce
Compare
Description
This PR includes the initial implementation of the GitLab trusted publisher. It's mostly based on the existing GitHub implementation, with a few modifications (mentioned below). Unit tests have been included, and a separate PR for the docs is available here. This is part of Trusted publishing: support for GitLab CI.
The corresponding GitLab feature (along with all the claims provided in the token) is documented here.
Differences with GitHub OIDC
org
part ofgithub.com/org/repo
, GitLab uses the term "namespace". A namespace can be either a username or a group. Groups can be nested, and so forhttps://gitlab.com/my_org/my_suborg/my_repo
, the namespace would bemy_org/my_suborg
.security-model.md
doc.workflow.yml
) can be anywhere in the GitLab repo. Since the OIDC claims include the path of the workflow relative to the root of the repo, when setting up a GitLab publisher we also ask for this path (as opposed to GitHub, where we only ask for the file name).TODO
Screenshot